Don't create unmet demand file unless needed.#1405
Open
dalonsoa wants to merge 5 commits into
Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1405 +/- ##
==========================================
+ Coverage 89.42% 89.44% +0.01%
==========================================
Files 59 59
Lines 8367 8381 +14
Branches 8367 8381 +14
==========================================
+ Hits 7482 7496 +14
Misses 564 564
Partials 321 321 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
tsmbland
approved these changes
Jul 10, 2026
tsmbland
left a comment
tsmbland
left a comment
Collaborator
There was a problem hiding this comment.
Looks good, just one small comment
| // If the unmet demand writer already exist, we return with an error, as it should not happen | ||
| ensure!( | ||
| self.unmet_demand_writer.is_none(), | ||
| "Unmet demand file already exists!" |
Collaborator
There was a problem hiding this comment.
If it's not possible for a user to trigger this, it should panic rather than returning an error
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts debug output generation so debug_unmet_demand.csv is only created when unmet demand is actually present, avoiding an always-empty debug file in normal successful runs.
Changes:
- Refactors
DebugDataWriterto store the unmet-demand file path and make the unmet-demand CSV writer optional (lazy creation). - Updates unmet-demand writing logic to no-op when there are no rows, and updates flushing to handle the optional writer.
- Updates regression test data for the
simplecase by removing the previously committed emptydebug_unmet_demand.csv.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/output.rs | Lazily creates and conditionally flushes the unmet-demand debug CSV writer to avoid producing an empty file. |
| tests/data/simple/debug_unmet_demand.csv | Removes the committed empty debug output file from regression test data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+444
to
+457
| let rows: Vec<(&CommodityID, &RegionID, &TimeSliceID, Flow)> = | ||
| iter.collect::<Vec<(&CommodityID, &RegionID, &TimeSliceID, Flow)>>(); | ||
|
|
||
| if rows.is_empty() { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| // If the unmet demand writer already exist, we return with an error, as it should not happen | ||
| if self.unmet_demand_writer.is_some() { | ||
| panic!("Unmet demand file already exists!"); | ||
| } | ||
|
|
||
| self.unmet_demand_writer = Some(csv::Writer::from_path(&self.unmet_demand_file_path)?); | ||
| for (commodity_id, region_id, time_slice, value) in rows { |
Comment on lines
470
to
472
| } | ||
|
|
||
| Ok(()) |
| self.unmet_demand_writer.serialize(row)?; | ||
| self.unmet_demand_writer | ||
| .as_mut() | ||
| .expect("Writer does not exist (cannot not happen)") |
Replaced panic with assert to handle existing unmet demand writer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The path for the file is added to the structure, keeping the actual writer as an option and only created if needed. A check is added to verify that the writer does not exist, which would mean that the run did not stopped after finding unmet demand. The file is closed alongside all the other files, if needed. The regression tests files for the
simplecase has been amended to removed the empty file, not needed anymore.Fixes #1372
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks